fix(ci): use portable fallbacks for bash-4 builtins and GNU sed in hook scripts by jiengup · Pull Request #3694 · apache/iggy

jiengup · 2026-07-17T15:25:13Z

Rationale

Several hook scripts use mapfile (bash >= 4) and sed -i without backup extension (GNU extension). On macOS stock bash 3.2 and BSD sed, these fail, blocking contributors from committing.

What changed

mapfile calls now guard with command -v mapfile and fall back to a while IFS= read -r loop.
sed -i calls is replaced by the sed -i.bak && rm pattern already established in trailing-whitespace.sh.

Local Execution

prek run -a

github-actions · 2026-07-17T15:25:34Z

Thanks for the PR. It is labeled S-waiting-on-review and queued for review.

Slash commands (own line, regular comment) move it around the queue:

/ready - back to S-waiting-on-review after addressing feedback
/author - flip to S-waiting-on-author while you finish changes
/request-review @user-or-team - request a reviewer

See CONTRIBUTING.md for details.

hubcio

the -i.bak switch itself is the right call and matches the trailing-whitespace.sh precedent. main ask is the && rm -f chaining - it quietly replaces the old loud sed failure with a false "Fixed" + exit 0 (details in the line comments).

two findings sit on lines outside this diff, so noting them here:

scripts/verify-crates-publish.sh:313 - lc="${crate,,}" is bash-4-only, stock macOS bash 3.2 dies with bad substitution, and it fails late (after all four crates already published to the local registry). it's also a no-op since the crate names are already lowercase, so lc="$crate" fixes it for free.
scripts/ci/python-sdk-version-sync.sh:144 - sort -V is GNU coreutils-only, BSD sort rejects it. the --fix path hits it exactly when the two versions differ, i.e. the case the hook exists for. worth confirming on an actual mac.

with a\, \b and these two still around, #3693 ends up only partially closed for macos contributors - either keep the issue open or note a follow-up in the PR.

jiengup · 2026-07-23T12:27:16Z

the -i.bak switch itself is the right call and matches the trailing-whitespace.sh precedent. main ask is the && rm -f chaining - it quietly replaces the old loud sed failure with a false "Fixed" + exit 0 (details in the line comments).

two findings sit on lines outside this diff, so noting them here:

scripts/verify-crates-publish.sh:313 - lc="${crate,,}" is bash-4-only, stock macOS bash 3.2 dies with bad substitution, and it fails late (after all four crates already published to the local registry). it's also a no-op since the crate names are already lowercase, so lc="$crate" fixes it for free.

scripts/ci/python-sdk-version-sync.sh:144 - sort -V is GNU coreutils-only, BSD sort rejects it. the --fix path hits it exactly when the two versions differ, i.e. the case the hook exists for. worth confirming on an actual mac.

with a\, \b and these two still around, #3693 ends up only partially closed for macos contributors - either keep the issue open or note a follow-up in the PR.

Appreciate your detailed review.

Problems fixed in the newest patch:

all the sed -i.bak ... && rm xxx.bak command is separated now, just like trailing-whitespace.sh does.
scripts/ci/sync-python-interpreter-version.sh: a\ GNU extension in sed proved not to work, and is now replaced by an awk insert.
scripts/verify-crates-publish.sh: add rm -f Cargo.toml.bak inside the CARGO_TOML_PATCHED branch of cleanup().
unnecessary mapfile fallback is removed, and the necessary ones are replaced by bare while IFS= read -r loop to reduce duplication.

Additional change:

scripts/verify-crates-publish.sh: lc="${crate,,}" changes to `lc="crate" and lower-case crate name contract is documented.
python-sdk-version-sync.sh: use sort -t. -k1,1n -k2,2n -k3,3n instead of sort -V, though sort -V is actually passed on macOS with bash 3.2.

Leave as a follow-up issue:

scripts/ci/sync-rustc-version.sh: \brust makes FROM-form dockerfiles get skipped entirely. A new follow-up issue is opened ci bash script relies on GNU-only \b regex syntax #3739. I will track it.

jiengup · 2026-07-23T12:27:50Z

/ready

ethanlin01x

Left a few minor comments inline. I ran into this problem myself on macOS, so I'd love to see this merged soon — thanks for the fix!

ethanlin01x · 2026-07-30T16:41:27Z

+    # Compare base versions using POSIX sort (X.Y.Z numeric per component).
    local sorted
-    sorted=$(printf '%s\n%s' "$base1" "$base2" | sort -V | head -1)
+    sorted=$(printf '%s\n%s' "$base1" "$base2" | sort -t. -k1,1n -k2,2n -k3,3n | head -1)


sort -V works fine on stock macOS (verified locally). Since the original way isn't broken and numeric keys only fit pure numbers, let's revert to -V. consider reverting to -V.

ethanlin01x · 2026-07-30T16:47:51Z

+            awk -v major="$PYTHON_VERSION_MAJOR" -v version="$PYTHON_VERSION" '
+              { print }
+              $0 == "    \"Programming Language :: Python :: " major "\"," {
+                printf "    \"Programming Language :: Python :: %s\",\n", version
+              }
+            ' "$file" > "$file.new" && mv "$file.new" "$file"


Could you split the awk and mv into two statements? With &&, a failed awk is exempt from set -e, so the script would still print "Fixed" and exit 0 with the file unedited — same errexit swallow as the earlier sed -i.bak ... && rm sites.

github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Jul 17, 2026

hubcio requested changes Jul 23, 2026

View reviewed changes

github-actions Bot added S-waiting-on-author PR is waiting on author response and removed S-waiting-on-review PR is waiting on a reviewer labels Jul 23, 2026

fix(ci): use non-bash shell compatible command for pre-commit hooks

66fb253

jiengup mentioned this pull request Jul 23, 2026

ci bash script relies on GNU-only \b regex syntax #3739

Open

2 tasks

jiengup requested a review from hubcio July 23, 2026 12:27

github-actions Bot added S-waiting-on-review PR is waiting on a reviewer and removed S-waiting-on-author PR is waiting on author response labels Jul 23, 2026

hubcio previously approved these changes Jul 23, 2026

View reviewed changes

jiengup dismissed hubcio’s stale review via 66fb253 July 23, 2026 12:29

jiengup force-pushed the ci-mapfile branch from ea3124c to 66fb253 Compare July 23, 2026 12:29

jiengup requested a review from hubcio July 23, 2026 12:30

ethanlin01x reviewed Jul 30, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(ci): use portable fallbacks for bash-4 builtins and GNU sed in hook scripts - #3694

fix(ci): use portable fallbacks for bash-4 builtins and GNU sed in hook scripts#3694
jiengup wants to merge 1 commit into
apache:masterfrom
jiengup:ci-mapfile

jiengup commented Jul 17, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 17, 2026

Uh oh!

hubcio left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jiengup commented Jul 23, 2026

Uh oh!

jiengup commented Jul 23, 2026

Uh oh!

ethanlin01x left a comment

Uh oh!

ethanlin01x Jul 30, 2026

Uh oh!

ethanlin01x Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

jiengup commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale

What changed

Local Execution

Uh oh!

github-actions Bot commented Jul 17, 2026

Uh oh!

hubcio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jiengup commented Jul 23, 2026

Uh oh!

jiengup commented Jul 23, 2026

Uh oh!

ethanlin01x left a comment

Choose a reason for hiding this comment

Uh oh!

ethanlin01x Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

ethanlin01x Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jiengup commented Jul 17, 2026 •

edited

Loading